Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Windows #494

Merged
merged 3 commits into from
Nov 22, 2024
Merged

Conversation

brainwavecoder9
Copy link
Contributor

What this does

Windows uses COM ports not what is expected for Linux/Mac. This commit adds support for Windows OS. It does this by:

  • checking to see if OS is Windows, or other
  • if Windows, use a new process
  • if other, use existing process

There is a dependency (pyserial) but it is already in the environment so no changes there. Once the port is identified it can be added to the .yaml the same as it is today. Although it will be referenced differently (e.g., COM7 vs. /dev/ttyACM) the rest of the functionality still works.

How it was tested

Successful in Windows environment, and theoretically won't affect others, but does need to be confirmed before merge.

How to checkout & try? (for the reviewer)

No change to existing process. To use, simply execute the existing script, e.g., python lerobot/scripts/find_motors_bus_port.py

@Cadene
Copy link
Collaborator

Cadene commented Oct 31, 2024

Looks good to me! Thanks for the improvements to print + comments ;)
Running tests and merging.

@Cadene
Copy link
Collaborator

Cadene commented Oct 31, 2024

Style check failing. Run our pre-commit ;)

pre-commit run --all-files

https://github.com/huggingface/lerobot/blob/main/CONTRIBUTING.md#submitting-a-pull-request-pr

@brainwavecoder9
Copy link
Contributor Author

Oh you mean I'm actually supposed to follow the instructions for contributing 😅

I thought "no way this won't pass" but here we are... updates made!

@brainwavecoder9
Copy link
Contributor Author

OK @Cadene this one isn't on me my test passed 😄

System.IO.IOException: No space left on device : '/home/runner/runners/2.320.0/_diag/Worker_20241101-143253-utc.log'
Unhandled exception. System.IO.IOException: No space left on device : '/home/runner/runners/2.320.0/_diag/Worker_20241101-143253-utc.log'

If this is something you want me to have a look at let me know, I've done lots of infra.

Copy link
Collaborator

@Cadene Cadene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Cadene Cadene merged commit 8af6935 into huggingface:main Nov 22, 2024
6 checks passed
@Cadene
Copy link
Collaborator

Cadene commented Nov 22, 2024

@brainwavecoder9 If you find any other compatibility issue, please let me know. I will be more reactive this time ;)

ChorntonYoel pushed a commit to ChorntonYoel/lerobot that referenced this pull request Nov 22, 2024
astroyat pushed a commit to astroyat/lerobot that referenced this pull request Nov 23, 2024
astroyat pushed a commit to astroyat/lerobot that referenced this pull request Nov 23, 2024
DomThePorcupine pushed a commit to DomThePorcupine/lerobot that referenced this pull request Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants